Skip to content

fix(harness): drop --auto from ship-pr; add review gate between CI and merge#64

Merged
fullstackjam merged 4 commits into
mainfrom
fix/ship-pr-review-gate
May 16, 2026
Merged

fix(harness): drop --auto from ship-pr; add review gate between CI and merge#64
fullstackjam merged 4 commits into
mainfrom
fix/ship-pr-review-gate

Conversation

@fullstackjam
Copy link
Copy Markdown
Collaborator

Summary

The previous ship-pr skill ended with gh pr merge --auto --squash, which queues the merge to fire as soon as branch-protection checks pass. That defeats the purpose of having a review step — auto-merge bypasses review entirely.

Corrected flow:

push → gh pr creategh pr checks --watcharchitecture-review the diff → surface findings → ask user → gh pr merge --squash (no --auto) → local cleanup

Two layers of gating, both intentional:

  • Mechanical — branch protection / 6 required checks (already enforced by GitHub, see docs/MERGE_POLICY.md).
  • Inferential — review of the diff after CI, before merge. Previously oral tradition; now encoded.

AGENTS.md and docs/HARNESS.md updated to match.

Test plan

  • Wait for CI to be green on this PR.
  • Review the diff via architecture-review (docs-only change; should be uneventful).
  • Surface findings + ask before merging — i.e. demonstrate the new flow on the PR that defines the new flow.

… and merge

The previous ship-pr flow ended with `gh pr merge --auto --squash`, which
queues the merge to fire as soon as branch-protection checks pass. That
defeats the purpose of having a review step — auto-merge bypasses
review entirely.

The corrected flow:

  push → gh pr create → gh pr checks --watch → architecture-review the
  diff → surface findings → ask user → gh pr merge --squash (no --auto)
  → local cleanup

Branch protection (docs/MERGE_POLICY.md) is the mechanical gate; this
skill adds the inferential gate on top of it. Skipping the gate by
queueing auto-merge was the wrong default.

AGENTS.md and docs/HARNESS.md updated to match the new flow.
@github-actions github-actions Bot added the docs label May 16, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Adds Step 6 "Triage the findings" between review and asking the user.

Findings split into:
- Self-fixable (typos, formatting, missing tests for branches this PR
  adds, bug fixes that don't change behaviour, following through on a
  rule the user already stated in-session) → fix in-session, loop back
  to Step 4. Do not burn the user's attention on them.
- Needs user judgment (design, scope, anything touching a prior
  deliberate choice) → surface, stop.

Without this layer, the previous skill would either silently fix
everything (over-eager) or escalate every typo (over-cautious). The
triage rule comes from the user's instruction in-session and is itself
the recursive case — fixing the missing triage rule, in the same PR
that adds the rule.
Previous Step 7 always handed the merge decision to the user, even when
review was clean. That defeats the harness loop: asking when there is
nothing to decide just burns the user's attention.

Triage now has three branches, not two:
- Needs user judgment → escalate, stop.
- Self-fixable → fix in-session, loop back to Step 4.
- Clean → merge directly. No confirmation prompt.

User explicitly stated this in-session ("没啥问题,也没啥需要我知道的,
你自己就合并了") — codifying it so the next agent doesn't repeat the
over-cautious default.

AGENTS.md and docs/HARNESS.md pointer text updated to match.
Renumbering left a 'Step 7' reference in the 'What NOT to do' entry
pointing at Merge instead of Triage. Escalation happens inside Step 6.
@fullstackjam fullstackjam merged commit 7d6b95e into main May 16, 2026
13 checks passed
@fullstackjam fullstackjam deleted the fix/ship-pr-review-gate branch May 16, 2026 14:14
fullstackjam added a commit that referenced this pull request May 16, 2026
…65)

The sensor in .claude/hooks/session-start.sh was introduced when ship-pr
used `gh pr merge --auto` — merges happened asynchronously after the
session ended, so a feature branch could linger checked out into the
next session and the sensor nagged the user to clean it up.

PR #64 dropped --auto. Merges are now synchronous and cleanup is Step 8
of the ship-pr loop, which always runs before the session can end on a
shipped PR. The sensor's trigger condition can no longer occur in
normal flow, so it's dead code paying for a per-session `git fetch`.

- session-start.sh restored to the pre-#63 shape (cache warming only).
- ship-pr SKILL.md drops the 'sensor will catch lingering branches'
  fallback paragraph; the cleanup step is now described as part of the
  loop, not optional.
- docs/HARNESS.md drops the sensor row from the controls table and adds
  a note in 'What's intentionally NOT in the harness' explaining the
  removal — so a future agent doesn't re-add it without also re-adding
  --auto.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant